chore: bump ESLint stack to v15 and migrate to flat config - #317
chore: bump ESLint stack to v15 and migrate to flat config#317cryptodev-2s wants to merge 7 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning MetaMask internal reviewing guidelines:
Ignoring alerts on:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6551ea9. Configure here.
6551ea9 to
311128a
Compare
311128a to
4adc549
Compare
16d390f to
9eb1d3a
Compare
Replaces .eslintrc.js with eslint.config.mjs and moves the whole ESLint
stack to the versions core uses.
eslint ^8.44.0 -> ^9.39.1
@metamask/eslint-config* ^12.0.0 -> ^15.0.0
@metamask/auto-changelog ^3.1.0 -> ^6.1.0
eslint-plugin-jest ^27.2.2 -> ^28.8.3
eslint-plugin-jsdoc ^39.9.1 -> ^50.2.4
eslint-plugin-n ^15.7.0 -> ^17.10.3
eslint-plugin-promise ^6.1.1 -> ^7.1.0
eslint-plugin-import removed, replaced by import-x
@typescript-eslint/{parser,plugin} removed, replaced by typescript-eslint
Added: typescript-eslint ^8.48.0, eslint-plugin-import-x ^4.3.0,
eslint-import-resolver-typescript ^3.6.3, @types/eslint ^9.6.1.
The flat config carries over the prettier/prettier and import-x/order
disables set in .eslintrc.js by the previous commit, since Oxfmt owns
formatting.
Also adds @types/semver ^7. It was previously reaching us transitively via
@typescript-eslint/eslint-plugin, so dropping that package broke the build
until it was declared directly.
Config v15 surfaced problems v12 never enforced. Most are stylistic and are
turned off with TODO comments, matching core. The rest were stale
eslint-disable directives pointing at rules typescript-eslint v8 renamed or
removed:
@typescript-eslint/no-throw-literal -> only-throw-error (8 sites)
@typescript-eslint/ban-types -> now unnecessary (3 sites)
jest/no-if -> removed in v28 (1 site)
Two autofixes were reverted because they changed behaviour rather than style:
hasProperty's constraint was rewritten from `Object` to `object`. The
original carried an explicit suppression, so it was deliberate: `Object`
accepts boxed primitives, and narrowing it would break callers passing a
string or number.
In json.test-d.ts the fixer stripped `as any` and `as number | undefined`
from tsd assertions. Those assertions are the test. no-unnecessary-type-
assertion is now off for *.test-d.ts so it cannot happen again.
jsdoc/require-jsdoc stays off for the same reason core keeps it off: its
autofixer inserts empty JSDoc blocks and mangles the surrounding code.
Verified: eslint, build, 23 suites / 1993 tests at 100% coverage, tsd, and
the full yarn lint all pass.
It was copied from smart-transactions-controller#595 without checking whether this package needs it. It does not: nothing here imports eslint types, eslint.config.mjs carries no annotations, and it is only an optional peer of eslint-plugin-prettier. Core does not have it at its root either. Removing it changes nothing: eslint, build, tests and lint all still pass.
0418ea3 to
cb576bc
Compare
Removing the stale eslint-disable comments left their newlines in place, so 16 stray blank lines ended up between JSDoc blocks and the declarations they document, and inside parameter lists. The cause is the autofixer for reportUnusedDisableDirectives: it deletes the directive text but not the line it sat on. I ran eslint --fix and did not review the whitespace only part of the diff. Nothing in the toolchain flags this. lines-around-comment, @typescript-eslint/lines-around-comment and no-multiple-empty-lines are all off in the shared config, and Oxfmt only collapses two or more consecutive blank lines, treating a single one as deliberate. Cosmetic rather than functional: TypeScript still associates a JSDoc block with the declaration across a blank line, verified by emitting declarations for both shapes and confirming the comment survives in the .d.ts.
| export * from './coercers'; | ||
| export * from './collections'; | ||
| export * from './encryption-types'; | ||
| export type * from './encryption-types'; |
There was a problem hiding this comment.
@typescript-eslint/consistent-type-exports
…ion emit no-unnecessary-type-assertion flagged `JsonRpcErrorStruct as Struct<JsonRpcError>` and its autofixer removed it. The rule is right that the assertion does not change assignability, and wrong that it is unnecessary: it pins what TypeScript emits. Without it the declaration inlines the structure instead of referring to the named type, so JsonRpcFailure and JsonRpcResponse changed from error: JsonRpcError; to an anonymous object literal, which also pushed the unexported internal ExactOptionalGuard further into the published types (9 occurrences in json.d.cts before, 11 after). Restored with a suppression explaining why. json.d.cts is now byte identical to the one built from main. Found by diffing the emitted declarations against main rather than by any test: assignability is unchanged, so nothing fails. Only the shape of the published types differs.
mcmire
left a comment
There was a problem hiding this comment.
Thanks for doing this. This mostly looks good but I called out some things below. I need to do another pass on this but I do have a suggestion to consider below for now.
| /** | ||
| * Predefined sizes (in Bytes) of specific parts of JSON structure. | ||
| */ | ||
| /* eslint-disable @typescript-eslint/no-duplicate-enum-values -- |
There was a problem hiding this comment.
This is fine for now. When we ban enums finally this should go away, so nothing to worry about.
| rules: { | ||
| // TODO: Re-enable these rules. | ||
| // Newly surfaced by eslint-config v15; not enforced under v12. | ||
| '@typescript-eslint/explicit-function-return-type': 'off', |
There was a problem hiding this comment.
Hmm, I thought I fixed this already. I guess not. This is okay for now.
There was a problem hiding this comment.
I don't follow why we're disabling this. It will be required for the migration to core, no?
| { | ||
| files: ['**/*.test.{js,ts}'], | ||
| rules: { | ||
| // These tests deliberately reach for `crypto` and `crypto.webcrypto`, |
There was a problem hiding this comment.
Probably okay, but I need to double-check this is the right thing to do here.
There was a problem hiding this comment.
The real reason is described here 8555d3c
0619330 to
4fb3fb6
Compare
|
@SocketSecurity ignore npm/@emnapi/core@1.10.0 |

Stacked on #314.
Replaces
.eslintrc.jswith a flateslint.config.mjsand moves the ESLint stack to core's versions.eslint^8.44.0^9.39.1@metamask/eslint-config{,-jest,-nodejs,-typescript}^12.0.0^15.0.0@metamask/auto-changelog^3.1.0^6.1.0eslint-plugin-jest^27.2.2^28.8.3eslint-plugin-jsdoc^39.9.1^50.2.4eslint-plugin-n^15.7.0^17.10.3eslint-plugin-promise^6.1.1^7.1.0Drops
eslint-plugin-importand@typescript-eslint/{parser,eslint-plugin}foreslint-plugin-import-xandtypescript-eslint. Adds@types/semver@^7, which had been arriving transitively through@typescript-eslint/eslint-pluginand broke the build once that was removed.Config v15 surfaced 351 problems v12 never enforced. Stylistic ones are off with TODO comments, matching core. The rest were stale directives naming rules that typescript-eslint v8 renamed or removed, so the code under them was going unchecked:
@typescript-eslint/no-throw-literalonly-throw-error@typescript-eslint/ban-typesimport/no-nodejs-modulesimport-x/no-nodejs-modulesjest/no-ifTwo autofixes are reverted because they changed behaviour rather than style:
hasProperty's constraint was rewritten fromObjecttoobject. The original carried an explicit suppression, soObjectwas deliberate: it accepts boxed primitives, and narrowing it breaks callers passing a string or number.json.test-d.tslostas anyandas number | undefinedfrom its tsd assertions. In type tests the assertion is the test.no-unnecessary-type-assertionis now off for*.test-d.ts.jsdoc/require-jsdocstays off rather than suppressed: its autofixer inserted 193 empty JSDoc blocks on the first pass. Core disables it for the same reason.Note
Medium Risk
Large toolchain bump touches most of the repo; export and
toWeiparsing changes could affect consumers, though runtime logic changes appear limited and intentional.Overview
Migrates linting from
.eslintrc.jsto flateslint.config.mjs, bumps ESLint 8 → 9 and@metamask/eslint-configv12 → v15, and swapseslint-plugin-import/ split@typescript-eslint/*foreslint-plugin-import-xandtypescript-eslint. Thelint:eslintscript drops--ext js,ts;@types/semveris added explicitly after dropping the old typescript-eslint transitive dep; LavaMoat allowseslint-plugin-import-x>unrs-resolver.The new config turns off several JSDoc rules (matching core, with TODOs), disables
no-unnecessary-type-assertionfor*.test-d.ts, and relaxes Node builtin checks in tests for crypto polyfill coverage.reportUnusedDisableDirectives: 'error'forces cleanup of stale eslint-disable comments across fixtures and tests.Source edits are mostly rule compliance: explicit return types, renamed disables (
only-throw-error,import-x/no-nodejs-modules),uuidnamed import infs.ts,export type *for type-only barrels, and typing tweaks inFrozenMap/FrozenSet. Two autofix reversals preserve behavior:hasPropertykeepsObject(boxed primitives), and type-test assertions stay intact.toWeinow treats empty whole/fraction parts (e.g..5,5.) as0, not onlyundefined.Reviewed by Cursor Bugbot for commit 4fb3fb6. Bugbot is set up for automated code reviews on this repo. Configure here.